Skip to content

fix(expression): report a character outside the grammar without closing the connection - #567

Open
LeeroyHannigan wants to merge 5 commits into
mainfrom
fix/tokenizer-multibyte
Open

LeeroyHannigan wants to merge 5 commits into
mainfrom
fix/tokenizer-multibyte

Conversation

@LeeroyHannigan

Copy link
Copy Markdown
Collaborator

A character outside the expression grammar, such as é, could close the connection instead of answering a syntax error. It now answers the syntax error DynamoDB gives, with the same token and window.

What happened on main

The tokenizer behind condition and filter expressions read the offending byte as a char and sliced the input five bytes either side of it. When that range ended inside a multi-byte character (#v = ééé, #v = 日本語), the slice panicked, the request task ended and the client saw the connection close. This reached:

  • ConditionExpression on PutItem, UpdateItem, DeleteItem and TransactWriteItems;
  • FilterExpression on Scan and Query;
  • KeyConditionExpression on Query, through the server's dynamodb:LeadingKeys parse, which runs before the authorization decision for every caller.

Other multi-byte input on those paths answered a token made of the first byte read as Latin-1 (token: "Ã") with a byte window. The tokenizer behind UpdateExpression, ProjectionExpression and the engine's key condition parse did not panic; it answered the character with one byte after it as the window (near: "é", near: "$x").

What the service answers

Measured on 2026-10-25 at 16:09, 16:34, 16:38, 16:43 and 16:54 UTC in us-east-1, each request sent twice with the same answer, on all five expression kinds. When the character is the first fault in the expression:

  • The token is the character as sent, a whole code point: é, 日, 😀, or $, @ and ! for ASCII characters outside the grammar.
  • The window runs from the start of the previous token to the end of the next one, keeping the whitespace between them. Each character outside the grammar counts as one token, and a name stops at the first such character:
    • #v = ééé answers token: "é", near: "= éé";
    • abéc = :v answers near: "abéc";
    • #v = é abc answers near: "= é abc";
    • #v é<> :v answers near: "#v é<>";
    • contains(#v, é) answers near: ", é)".
  • When no token comes before the character, the window starts at the start of the expression: é = :v answers near: " é =".
  • U+0000 is removed from the token and the window: #v = \u0000x answers token: "", near: "= x".

Both tokenizer entry points now follow this rule.

Tests

  • crates/core/src/expression/tokenizer_tests.rs: one table test with 44 measured rows, run through both entry points. Each of these altered versions of the change fails it: the window starting at the character, the window not reaching the next token, the token read as a byte, no two-character operators, no name runs, no digit runs, and U+0000 kept.
  • tests/test_expression_unknown_character.py: 50 dual-target wire tests on Scan and Query filters, PutItem and UpdateItem conditions, Query key conditions, UpdateItem update expressions and GetItem projections. The Query key condition rows include the shape that closed the connection on main.

Run locally on 2026-10-25 against a SQLite release build:

  • the new file: 50 passed at the head, 50 failed on main;
  • the 11 wire files that assert expression syntax errors, plus the new file: 431 passed at the head, and the server log has no panic;
  • cargo +1.97.0 fmt --all -- --check, cargo +1.97.0 clippy -p extenddb-core --all-targets -- -D warnings, and cargo test -p extenddb-core on stable (521 passed) and on 1.88.0 (520 passed).

The full SQLite wire suite at the third commit had 2 failures, in tests/test_gsi_async.py, which starts a server from ./target/release/extenddb, a path that did not exist in that run; main fails the same 2. CI runs the new file in run-integration, run-integration-sqlite, mongodb-pytest and cassandra-pytest.

Not changed here

These differences from the service are on main and stay as they are:

  • When a parser fault comes before the character (#v = = é), the service reports that fault and ExtendDB reports the character, because ExtendDB tokenizes the whole expression before parsing.
  • # or : followed by a character that cannot start a name keeps main's texts (empty name reference '#', empty value placeholder ':'). The service answers token: "#", near: "#é" by the same window rule.
  • The service strips one leading byte-order mark and accepts the expression; ExtendDB refuses it.
  • #v = 1: the service refuses a bare number as an operand; ExtendDB reads it as an attribute name.
  • The expression size error: the service no longer appends ; expression size: <n> on filter and condition expressions; ExtendDB does.
  • The 1 validation error detected: prefix the service adds on write paths.

Not measured: the service's answer to a caller fenced by dynamodb:LeadingKeys for such a key condition. ExtendDB answers ValidationException under ForAllValues and AccessDeniedException under ForAnyValue, with no items either way.

Files

tokenizer.rs passed 600 lines with the fix, so the second commit moves its tests into tokenizer_tests.rs with no other change, using the #[path] pattern of evaluator_tests.rs. tokenizer.rs is now 449 lines.

Merging

git merge-tree --write-tree is clean against #558, #511 and #479. #558 builds a window for parser faults from the same rule. Its window also starts at the first token when nothing comes before the fault, where the service starts at the start of the expression; that is fixed on #558 separately.

…e does, without a panic

The tokenizer behind condition and filter expressions read the offending
byte as a char and sliced the input five bytes either side of it. When that
range ended inside a multi-byte character (`#v = ééé`, `#v = 日本語`) the
slice panicked, the request task ended and the connection closed; the
server's key condition parse ahead of authorization calls the same tokenizer,
so a Query key condition did the same for every principal. Other multi-byte
input answered a token made of the first byte read as Latin-1. The tokenizer
behind update, projection and the engine's key condition parse did not panic
and answered the character with one byte after it as the window.

The error now names the character as sent and shows the service's window:
the source text from the start of the previous token to the end of the next
one, with each character outside the grammar counted as one token (measured
2026-10-25, each request twice: `#v = ééé` answers `token: "é", near: "= éé"`,
`abéc = :v` answers `near: "abéc"`, `#v = $x` answers `near: "= $x"`). Both
tokenizer entry points share the rule.
Pure move, using the #[path] pattern evaluator_tests.rs uses; tokenizer.rs
had grown past 600 lines with the previous commit.
…rammar

Every row is a request measured against the service on 2026-10-25, each sent
twice: Scan filters with multi-byte and ASCII characters outside the grammar
in each position, and one or two rows each for PutItem conditions, Query key
conditions, UpdateItem update expressions and GetItem projections.
…drop U+0000, as the service does

When no token comes before the character, the service starts the window at
the start of the expression and keeps the leading whitespace (`  é = :v`
answers `near: "  é ="`), and it removes U+0000 from the token and the
window (`#v = \0x` answers `token: "", near: "= x"`). Measured 2026-10-25 at
16:34, 16:38 and 16:43 UTC, each request twice. The unit table gains those
rows and the rows that pin the next-token rules for two-character operators,
name runs and digit runs.
…ken rules and the Query paths

Rows measured on 2026-10-25 at 16:34, 16:38, 16:43 and 16:54 UTC, each
request twice: a character with no token before it, U+0000, two-character
operators, name runs and digit runs after the character, and the panic shape
on a Query key condition, a Query filter and an UpdateItem condition. The
prefix the service adds on write paths is stripped only on those paths.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants